Implement complete auth UI and integrate Focus feature set#27
Merged
Implement complete auth UI and integrate Focus feature set#27
Conversation
…o feature/auth-api
- Added Pomodoro timer with Start/Reset/Skip logic. - Integrated local Quick Notes with Pin/Delete functionality. - Supported image attachments in notes using image_picker. - Added Focus settings: time duration, vibration, and ringtones.
Contributor
📝 WalkthroughWalkthroughDeleted login/register/forgot-password screens; changed auth viewmodels to return error strings and added dispose(); OTP length increased to 8 and strings switched to English; added Focus (Pomodoro) feature with notes, image pick, persistence, and NoteModel; wired FocusScreen into main tabs; added flutter_ringtone_player, image_picker, shared_preferences; minor manifest/comment edits. Changes
Sequence DiagramsequenceDiagram
actor User
participant FocusScreen
participant FocusViewModel
participant ImagePicker
participant FlutterRingtone
participant SharedPrefs
User->>FocusScreen: Open Focus tab
FocusScreen->>FocusViewModel: subscribe / read state
rect rgba(100,150,200,0.5)
Note over User,FocusViewModel: Timer flow
User->>FocusScreen: Toggle start/stop
FocusScreen->>FocusViewModel: toggleTimer()
FocusViewModel->>FocusViewModel: run countdown (periodic)
alt Timer reaches zero
FocusViewModel->>FlutterRingtone: play ringtone
FocusViewModel->>User: Haptic feedback
FocusViewModel->>FocusViewModel: set isRinging
end
end
rect rgba(150,100,200,0.5)
Note over User,FocusViewModel: Quick note flow
User->>FocusScreen: Tap attach image
FocusScreen->>ImagePicker: pickImage()
ImagePicker-->>FocusScreen: image path
FocusScreen->>FocusViewModel: pickImage() / set selectedImagePath
User->>FocusScreen: Save note
FocusScreen->>FocusViewModel: addNote()
FocusViewModel->>SharedPrefs: persist notes (toJson)
FocusViewModel->>FocusViewModel: notifyListeners()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/features/auth/viewmodels/auth_viewmodels.dart (1)
14-29:⚠️ Potential issue | 🟡 MinorMissing
TextEditingControllerdisposal causes memory leaks.
LoginViewModelcreatesTextEditingControllerinstances that are never disposed. When using these controllers withChangeNotifier, you should overridedispose()to clean up.🛡️ Proposed fix
class LoginViewModel extends BaseViewModel { final emailCtrl = TextEditingController(); final passCtrl = TextEditingController(); bool obscurePass = true; void togglePass() { obscurePass = !obscurePass; notifyListeners(); } + `@override` + void dispose() { + emailCtrl.dispose(); + passCtrl.dispose(); + super.dispose(); + } + Future<String?> login() async {The same pattern should be applied to
RegisterViewModel,ForgotPassViewModel, andNewPassViewModel.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/auth/viewmodels/auth_viewmodels.dart` around lines 14 - 29, LoginViewModel and other viewmodels (RegisterViewModel, ForgotPassViewModel, NewPassViewModel) create TextEditingController instances (emailCtrl, passCtrl, etc.) but never dispose them; override the dispose() method on each viewmodel class to call emailCtrl.dispose(), passCtrl.dispose(), (and any other controllers) and then call super.dispose() so controllers are properly cleaned up and prevent memory leaks.
🧹 Nitpick comments (3)
src/lib/features/auth/services/auth_helper.dart (1)
136-143: Consider movingsignOut()into theAuthHelperclass.The
signOut()function is defined as a top-level function outside theAuthHelperclass, while all other auth operations are class methods. This breaks consistency and makes the API surface harder to discover.♻️ Proposed refactor
-// SignOut session -Future<void> signOut() async { - try { - await supabase.auth.signOut(); - } catch (e) { - print('Lỗi đăng xuất: $e'); - rethrow; - } -}Add inside
AuthHelperclass:class AuthHelper { // ... existing methods ... Future<void> signOut() async { try { await supabase.auth.signOut(); } catch (e) { print('Lỗi đăng xuất: $e'); rethrow; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/auth/services/auth_helper.dart` around lines 136 - 143, The signOut() function is currently a top-level function and should be moved into the AuthHelper class for consistency; locate the standalone signOut() implementation that calls supabase.auth.signOut() and relocate its body into the AuthHelper class as an instance method named signOut(), removing the top-level definition so callers use AuthHelper().signOut() (or the existing AuthHelper instance) and keep the existing try/catch and rethrow behavior unchanged.src/lib/features/auth/viewmodels/auth_viewmodels.dart (1)
40-46: Missing input validation inregister()compared to production version.This mock
RegisterViewModel.register()only checks password match but lacks:
- Empty field validation for name/email
- Email format validation
- Password strength validation
The production version at
presentation/viewmodels/auth_viewmodels.dartincludes comprehensive validation. If this file is used in production, consider aligning validation rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/auth/viewmodels/auth_viewmodels.dart` around lines 40 - 46, The register() method currently only checks password confirmation; update RegisterViewModel.register() to mirror production validation by adding checks for empty name/email (e.g., nameCtrl.text, emailCtrl.text), validating email format (use the same regex or EmailValidator logic from presentation/viewmodels/auth_viewmodels.dart), and enforcing password strength rules (minimum length and required character types consistent with production) before setLoading and the artificial delay; return specific error strings for each failing validation and only proceed to setLoading/return null when all validations pass.src/lib/features/note/viewmodel/focus_viewmodel.dart (1)
132-133: Timer tick notifications are too broad for this shared ViewModel.
notifyListeners()every second will rebuild allFocusViewModelconsumers (including note input/list), which can hurt typing smoothness and scrolling. Consider splitting timer and note state (or usingSelector/ValueNotifierfor timer-only updates).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/features/note/view/focus_screen.dart`:
- Around line 27-57: Replace hardcoded UI strings in the settings dialog with
localized resources: move "Settings", "Sound", "Vibrate" (and the dialog action
buttons "Cancel" and "Save" referenced around lines 81-89) into the app
localization files and use the localization getters in the widget tree (e.g.
replace Text('Settings') with something like
Text(AppLocalizations.of(context).settings), SwitchListTile title
Text('Vibrate') with Text(AppLocalizations.of(context).vibrate), and the action
buttons with AppLocalizations cancel/save labels). Update references in this
file (the Slider labels using currentPomodoro/currentBreak and the Text widgets,
plus the SwitchListTile title) to use the localized strings and ensure imports
and localization context are available where needed.
In `@src/lib/features/note/viewmodel/focus_viewmodel.dart`:
- Line 112: The progress getter divides by totalTime without guarding against
non-positive values; update the progress getter to check totalTime > 0 and
return a safe default (e.g., 0.0) when totalTime <= 0 to avoid NaN/Infinity and
rendering issues, and apply the same guard to any other computed properties in
this class that divide by totalTime (refer to timeRemaining, totalTime and the
public updateSettings method that can set invalid durations) so invalid inputs
are clamped/handled before performing the division.
- Around line 128-130: The playback started in the ringtone branch (where
ringtoneType == 1/2/3) can loop indefinitely; add a class-level
FlutterRingtonePlayer instance (e.g., _ringtonePlayer) and use it for playback
instead of the current static calls, then call _ringtonePlayer.stop() from
resetTimer(), setMode(), and dispose() to ensure looping audio is stopped when
resetting, switching mode, or disposing the viewmodel; update the playback calls
to use the instance playAlarm/playNotification/playRingtone methods so stop()
will affect them.
In `@src/lib/features/tasks/view/screens/task_detail_screen.dart`:
- Line 27: The comment above the state initialization is misleading by saying
"services" when the code pulls properties directly from widget.task; update the
comment to accurately describe what happens (e.g., "Initialize state variables
from the passed task object") near the TaskDetailScreen State init code where
widget.task.title, widget.task.description, widget.task.startTime,
widget.task.endTime, and widget.task.category are accessed to make the intent
clear.
In `@src/pubspec.yaml`:
- Around line 42-43: Add the missing iOS usage-description keys required by
image_picker: update the iOS app Info.plist to include
NSPhotoLibraryUsageDescription (for gallery access) and NSCameraUsageDescription
(for camera access) and, if you support video/audio capture, also add
NSMicrophoneUsageDescription; set each key to a clear, user-facing string
explaining why the app needs that permission (e.g., "Allow access to your photos
to select images" / "Allow access to your camera to take photos or videos").
Ensure these keys are present whenever the image_picker dependency
(image_picker) is used so image picking succeeds on device and during app
review.
---
Outside diff comments:
In `@src/lib/features/auth/viewmodels/auth_viewmodels.dart`:
- Around line 14-29: LoginViewModel and other viewmodels (RegisterViewModel,
ForgotPassViewModel, NewPassViewModel) create TextEditingController instances
(emailCtrl, passCtrl, etc.) but never dispose them; override the dispose()
method on each viewmodel class to call emailCtrl.dispose(), passCtrl.dispose(),
(and any other controllers) and then call super.dispose() so controllers are
properly cleaned up and prevent memory leaks.
---
Nitpick comments:
In `@src/lib/features/auth/services/auth_helper.dart`:
- Around line 136-143: The signOut() function is currently a top-level function
and should be moved into the AuthHelper class for consistency; locate the
standalone signOut() implementation that calls supabase.auth.signOut() and
relocate its body into the AuthHelper class as an instance method named
signOut(), removing the top-level definition so callers use
AuthHelper().signOut() (or the existing AuthHelper instance) and keep the
existing try/catch and rethrow behavior unchanged.
In `@src/lib/features/auth/viewmodels/auth_viewmodels.dart`:
- Around line 40-46: The register() method currently only checks password
confirmation; update RegisterViewModel.register() to mirror production
validation by adding checks for empty name/email (e.g., nameCtrl.text,
emailCtrl.text), validating email format (use the same regex or EmailValidator
logic from presentation/viewmodels/auth_viewmodels.dart), and enforcing password
strength rules (minimum length and required character types consistent with
production) before setLoading and the artificial delay; return specific error
strings for each failing validation and only proceed to setLoading/return null
when all validations pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8934f0ea-b68c-4977-8c18-0185cf93faa1
⛔ Files ignored due to path filters (1)
src/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
src/android/app/src/main/AndroidManifest.xmlsrc/lib/features/auth/forgot_password_view.dartsrc/lib/features/auth/login_view.dartsrc/lib/features/auth/otp_verification_view.dartsrc/lib/features/auth/presentation/view/new_password_view.dartsrc/lib/features/auth/presentation/viewmodels/auth_viewmodels.dartsrc/lib/features/auth/register_view.dartsrc/lib/features/auth/services/auth_helper.dartsrc/lib/features/auth/viewmodels/auth_viewmodels.dartsrc/lib/features/main/view/screens/main_screen.dartsrc/lib/features/note/model/note_model.dartsrc/lib/features/note/view/focus_screen.dartsrc/lib/features/note/view/focus_widget.dartsrc/lib/features/note/viewmodel/focus_viewmodel.dartsrc/lib/features/tasks/view/screens/task_detail_screen.dartsrc/pubspec.yaml
💤 Files with no reviewable changes (4)
- src/android/app/src/main/AndroidManifest.xml
- src/lib/features/auth/login_view.dart
- src/lib/features/auth/register_view.dart
- src/lib/features/auth/forgot_password_view.dart
Comment on lines
+27
to
+57
| title: const Text('Settings', style: TextStyle(fontWeight: FontWeight.bold, color: Color(0xFF2C3E50))), | ||
| content: SingleChildScrollView( | ||
| child: Column( | ||
| mainAxisSize: MainAxisSize.min, | ||
| crossAxisAlignment: CrossAxisAlignment.start, | ||
| children: [ | ||
| Row( | ||
| mainAxisAlignment: MainAxisAlignment.spaceBetween, | ||
| children: [ | ||
| const Text('Pomodoro', style: TextStyle(fontWeight: FontWeight.bold, fontSize: 14)), | ||
| Text('${currentPomodoro.toInt()} min', style: const TextStyle(color: AppColors.primaryBlue, fontWeight: FontWeight.bold)), | ||
| ], | ||
| ), | ||
| Slider(value: currentPomodoro, min: 5, max: 60, divisions: 55, activeColor: AppColors.primaryBlue, onChanged: (val) => setStateDialog(() => currentPomodoro = val)), | ||
| const SizedBox(height: 5), | ||
| Row( | ||
| mainAxisAlignment: MainAxisAlignment.spaceBetween, | ||
| children: [ | ||
| const Text('Short Break', style: TextStyle(fontWeight: FontWeight.bold, fontSize: 14)), | ||
| Text('${currentBreak.toInt()} min', style: const TextStyle(color: Colors.orange, fontWeight: FontWeight.bold)), | ||
| ], | ||
| ), | ||
| Slider(value: currentBreak, min: 1, max: 30, divisions: 29, activeColor: Colors.orange, onChanged: (val) => setStateDialog(() => currentBreak = val)), | ||
| const Divider(height: 30), | ||
| SwitchListTile( | ||
| title: const Text('Vibrate', style: TextStyle(fontWeight: FontWeight.bold, fontSize: 14)), | ||
| value: currentVibrate, activeColor: AppColors.primaryBlue, contentPadding: EdgeInsets.zero, | ||
| onChanged: (val) => setStateDialog(() => currentVibrate = val), | ||
| ), | ||
| const SizedBox(height: 10), | ||
| const Text('Sound', style: TextStyle(fontWeight: FontWeight.bold, fontSize: 14)), |
Contributor
There was a problem hiding this comment.
Unify localization for settings labels/actions.
This dialog mixes English (Settings, Cancel, Save, Sound) with Vietnamese UI elsewhere. Please move these strings into the app localization layer for consistent UX.
Also applies to: 81-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/features/note/view/focus_screen.dart` around lines 27 - 57, Replace
hardcoded UI strings in the settings dialog with localized resources: move
"Settings", "Sound", "Vibrate" (and the dialog action buttons "Cancel" and
"Save" referenced around lines 81-89) into the app localization files and use
the localization getters in the widget tree (e.g. replace Text('Settings') with
something like Text(AppLocalizations.of(context).settings), SwitchListTile title
Text('Vibrate') with Text(AppLocalizations.of(context).vibrate), and the action
buttons with AppLocalizations cancel/save labels). Update references in this
file (the Slider labels using currentPomodoro/currentBreak and the Text widgets,
plus the SwitchListTile title) to use the localized strings and ensure imports
and localization context are available where needed.
| return '$minutes:$seconds'; | ||
| } | ||
|
|
||
| double get progress => timeRemaining / totalTime; |
Contributor
There was a problem hiding this comment.
Guard against non-positive durations in public settings API.
progress assumes totalTime > 0. Since updateSettings is public, invalid inputs can produce invalid progress values and break progress rendering.
Defensive fix
- double get progress => timeRemaining / totalTime;
+ double get progress => totalTime <= 0 ? 0 : (timeRemaining / totalTime).clamp(0.0, 1.0);
void updateSettings({required int newPomodoroMinutes, required int newBreakMinutes, required bool vibrate, required int ringtone}) {
+ if (newPomodoroMinutes <= 0 || newBreakMinutes <= 0) {
+ throw ArgumentError('Timer durations must be greater than 0 minutes');
+ }
pomodoroTime = newPomodoroMinutes * 60;Also applies to: 156-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/features/note/viewmodel/focus_viewmodel.dart` at line 112, The
progress getter divides by totalTime without guarding against non-positive
values; update the progress getter to check totalTime > 0 and return a safe
default (e.g., 0.0) when totalTime <= 0 to avoid NaN/Infinity and rendering
issues, and apply the same guard to any other computed properties in this class
that divide by totalTime (refer to timeRemaining, totalTime and the public
updateSettings method that can set invalid durations) so invalid inputs are
clamped/handled before performing the division.
| void initState() { | ||
| super.initState(); | ||
| // Initialize state variables with data from the passed task object | ||
| // Initialize state variables with services from the passed task object |
Contributor
There was a problem hiding this comment.
Correct the misleading comment.
The comment states "Initialize state variables with services from the passed task object," but the code below (lines 28-32) directly accesses properties (title, description, startTime, endTime, category) from widget.task, not services. The word "services" is incorrect and misleading.
📝 Proposed fix to restore accurate documentation
- // Initialize state variables with services from the passed task object
+ // Initialize state variables with data from the passed task object📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Initialize state variables with services from the passed task object | |
| // Initialize state variables with data from the passed task object |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/features/tasks/view/screens/task_detail_screen.dart` at line 27, The
comment above the state initialization is misleading by saying "services" when
the code pulls properties directly from widget.task; update the comment to
accurately describe what happens (e.g., "Initialize state variables from the
passed task object") near the TaskDetailScreen State init code where
widget.task.title, widget.task.description, widget.task.startTime,
widget.task.endTime, and widget.task.category are accessed to make the intent
clear.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/lib/features/note/view/focus_screen.dart (1)
27-37:⚠️ Potential issue | 🟡 MinorMove hardcoded UI strings into localization resources.
Hardcoded labels are still present at Line 27, Line 36, Line 45, Line 52, Line 57, Line 67–69, Line 81, Line 88, and Line 115, which keeps this screen inconsistent with localized flows.
Also applies to: 45-57, 67-69, 81-89, 115-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/note/view/focus_screen.dart` around lines 27 - 37, Replace all hardcoded UI strings in FocusScreen's dialog (e.g., the title "Cài đặt", the label "Pomodoro", the minutes suffix "phút" used with currentPomodoro, and the other hardcoded labels around the Pomodoro/short/long break controls and buttons) with localization lookups; locate the Text widgets in focus_screen.dart (search for the literal "Cài đặt", "Pomodoro", "phút" and other Vietnamese labels) and change them to use your app's localization API (for example S.of(context).settings, S.of(context).pomodoro, S.of(context).minutes, etc.), add the new keys to the ARB/JSON/localization resource files, and update any plural/interpolated strings (like "${currentPomodoro.toInt()} phút") to use localized formatting or a parameterized localized string to preserve correct grammar and number formatting.
🧹 Nitpick comments (1)
src/lib/features/note/view/focus_screen.dart (1)
118-121: Use an accessible button widget for the settings action.At lines 118–121,
GestureDetector+Containerdoes not provide built-in button semantics for screen readers.IconButtonis the recommended Flutter widget for accessible tappable icon actions and includes native support for tooltips that can be vocalized by assistive technology.The suggested refactor maintains the visual appearance (white background, circular shape, same icon) while improving accessibility:
Suggested change
- GestureDetector( - onTap: () => _showSettingsDialog(context), - child: Container(padding: const EdgeInsets.all(10), decoration: const BoxDecoration(color: Colors.white, shape: BoxShape.circle), child: const Icon(Icons.settings_outlined, color: AppColors.primaryBlue)), - ), + IconButton( + tooltip: 'Cài đặt', + onPressed: () => _showSettingsDialog(context), + style: IconButton.styleFrom( + backgroundColor: Colors.white, + padding: const EdgeInsets.all(10), + shape: const CircleBorder(), + ), + icon: const Icon(Icons.settings_outlined, color: AppColors.primaryBlue), + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/note/view/focus_screen.dart` around lines 118 - 121, Replace the non-semantic GestureDetector+Container used for the settings action with an accessible IconButton so screen readers get proper button semantics and tooltips; locate the GestureDetector wrapping the Icon(Icons.settings_outlined) that calls _showSettingsDialog(context) and change it to an IconButton that preserves the visual style (white circular background, padding, and AppColors.primaryBlue icon color) and provide a tooltip (e.g., "Settings") while wiring onPressed to call _showSettingsDialog.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/features/note/view/focus_screen.dart`:
- Around line 83-85: The “Lưu” button currently only updates runtime state via
vm.updateSettings(...) and does not persist across restarts; update the handler
so settings are saved to persistent storage rather than only in-memory: either
extend vm.updateSettings to also write to the app settings repository (e.g.,
call SettingsRepository.saveSettings or equivalent) or after
vm.updateSettings(...) call a persistence method (e.g.,
settingsRepository.save(...) or prefs.setInt/SetString for
currentPomodoro/currentBreak/vibrate/ringtone) before Navigator.pop(context);
ensure you use the same settings model used elsewhere so persisted values are
read on app startup.
---
Duplicate comments:
In `@src/lib/features/note/view/focus_screen.dart`:
- Around line 27-37: Replace all hardcoded UI strings in FocusScreen's dialog
(e.g., the title "Cài đặt", the label "Pomodoro", the minutes suffix "phút" used
with currentPomodoro, and the other hardcoded labels around the
Pomodoro/short/long break controls and buttons) with localization lookups;
locate the Text widgets in focus_screen.dart (search for the literal "Cài đặt",
"Pomodoro", "phút" and other Vietnamese labels) and change them to use your
app's localization API (for example S.of(context).settings,
S.of(context).pomodoro, S.of(context).minutes, etc.), add the new keys to the
ARB/JSON/localization resource files, and update any plural/interpolated strings
(like "${currentPomodoro.toInt()} phút") to use localized formatting or a
parameterized localized string to preserve correct grammar and number
formatting.
---
Nitpick comments:
In `@src/lib/features/note/view/focus_screen.dart`:
- Around line 118-121: Replace the non-semantic GestureDetector+Container used
for the settings action with an accessible IconButton so screen readers get
proper button semantics and tooltips; locate the GestureDetector wrapping the
Icon(Icons.settings_outlined) that calls _showSettingsDialog(context) and change
it to an IconButton that preserves the visual style (white circular background,
padding, and AppColors.primaryBlue icon color) and provide a tooltip (e.g.,
"Settings") while wiring onPressed to call _showSettingsDialog.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16ac0378-871f-445a-9818-e18f27e361ee
📒 Files selected for processing (2)
src/lib/features/auth/presentation/viewmodels/auth_viewmodels.dartsrc/lib/features/note/view/focus_screen.dart
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/lib/features/note/view/focus_widget.dart (2)
306-314: Consider adding semantic labels for accessibility.The
GestureDetectorwrapping the remove button lacks accessibility semantics. Users with screen readers won't understand the button's purpose.♿ Proposed accessibility improvement
-GestureDetector( - onTap: () => vm.removeNote(note.id), - child: Container( +Semantics( + label: 'Remove note', + button: true, + child: GestureDetector( + onTap: () => vm.removeNote(note.id), + child: Container( + // ...existing code + ), + ), +),Alternatively, consider using
IconButtonwhich has built-in accessibility support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/note/view/focus_widget.dart` around lines 306 - 314, The current remove control uses a GestureDetector wrapping a Container (calling vm.removeNote(note.id)) without accessibility semantics; update this by replacing or augmenting the GestureDetector/Container with an accessible widget (e.g., IconButton) or wrap it with a Semantics widget and provide a meaningful label/tooltip like "Remove note" and an appropriate onTap/action hint; ensure the handler still calls vm.removeNote(note.id) so behavior is unchanged and that accessibility/semantic properties (label, buttonRole) are set for screen readers.
20-20: ReplacewithOpacity()withwithValues(alpha:)for forward compatibility with Flutter 3.27+.The file contains 5 instances of the deprecated
Color.withOpacity()method (lines 20, 87, 99, 167, 168). UsewithValues(alpha: ...)instead to avoid precision loss and maintain compatibility with newer Flutter versions:// Before Colors.black.withOpacity(0.02) // After Colors.black.withValues(alpha: 0.02)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/note/view/focus_widget.dart` at line 20, Replace all uses of the deprecated Color.withOpacity(...) in this widget with Color.withValues(alpha: ...). Specifically, update the five occurrences inside the FocusWidget (the boxShadow entry and the four other Color.withOpacity(...) calls around the build/layout code) to use withValues(alpha: <same numeric>) to preserve precision and forward-compatibility with Flutter 3.27+; ensure you keep the same numeric alpha values and import stays unchanged.src/lib/features/note/view/focus_screen.dart (1)
118-118: Hardcoded avatar URL may cause network failures.The NetworkImage URL
https://i.pravatar.cc/150?u=a042581f4e29026704dwill fail if the external service is unavailable, causing a broken image. Consider using a local placeholder or adding anerrorBuilder.♻️ Proposed fix with error handling
-const CircleAvatar(radius: 22, backgroundImage: NetworkImage('https://i.pravatar.cc/150?u=a042581f4e29026704d')), +CircleAvatar( + radius: 22, + backgroundImage: const AssetImage('assets/images/default_avatar.png'), + onBackgroundImageError: (_, __) {}, +),Or use a local asset placeholder instead of relying on external service availability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/note/view/focus_screen.dart` at line 118, Replace the hardcoded external avatar URL usage in the CircleAvatar widget (where NetworkImage('https://i.pravatar.cc/...') is used) with robust handling: either switch to a local asset image placeholder or wrap the NetworkImage usage with an Image widget that provides an errorBuilder/fallback so a local placeholder displays when the network image fails; update the CircleAvatar to use that Image provider or AssetImage and ensure the placeholder asset is referenced (e.g., AssetImage or a named local asset) and included in pubspec so the avatar never shows a broken image.src/lib/features/note/viewmodel/focus_viewmodel.dart (1)
49-60: Image picker error handling is good, but consider user feedback.The try-catch logs the error but provides no user feedback. Consider showing a snackbar or toast to inform the user that image selection failed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/note/viewmodel/focus_viewmodel.dart` around lines 49 - 60, The catch block in pickImage currently only logs errors (debugPrint) and gives no user feedback; modify the method signature to accept a BuildContext (e.g., Future<void> pickImage(BuildContext context)) and, in the catch(e) branch, show a user-visible message (for example via ScaffoldMessenger.of(context).showSnackBar or a toast) indicating image selection failed, while keeping the existing behavior of not changing selectedImagePath and calling notifyListeners only on success; reference: pickImage, _picker, selectedImagePath, notifyListeners, debugPrint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/features/note/view/focus_widget.dart`:
- Around line 235-238: The Image.file usage inside the ClipRRect (in the widget
rendering code using vm.selectedImagePath) lacks error handling and will throw
if the file is missing; update the Image.file call to supply an errorBuilder
that handles missing/inaccessible files gracefully (e.g., show a placeholder
icon or SizedBox) and ensure you still pass height, width, and fit so the
fallback matches the intended layout; locate the ClipRRect/Image.file block and
add the errorBuilder parameter to prevent runtime exceptions when
vm.selectedImagePath is invalid.
- Around line 326-334: The Image.file usage inside ClipRRect
(Image.file(File(note.imagePath!), width: double.infinity, height: 150, fit:
BoxFit.cover)) can throw if the file is missing; update this widget to add an
errorBuilder to Image.file (same pattern used for the preview image) that
renders a fallback (e.g., an Icon or placeholder Container) when loading fails,
and ensure you also handle a null or empty note.imagePath before creating File
to avoid exceptions in the focus widget rendering pipeline.
In `@src/lib/features/note/viewmodel/focus_viewmodel.dart`:
- Around line 38-45: The loadNotesFromDisk method currently calls
NoteModel.fromJson on every saved string and will crash if any entry is
malformed; update loadNotesFromDisk to wrap the deserialization loop in a
try-catch, handle individual failures (e.g., skip the bad JSON, optionally log
the error or remove that entry), and only assign the successfully parsed
NoteModel instances to notes before calling notifyListeners so a single
corrupted item won't crash startup.
- Around line 69-85: The addNote(), removeNote(), and togglePin() methods call
the async saveNotesToDisk() without awaiting, risking interleaved writes; make
each of these methods async (e.g., declare async on
addNote/removeNote/togglePin) and await saveNotesToDisk() so disk writes run
sequentially, then call notifyListeners() after the awaited save; update any
call sites if they rely on synchronous behavior. Alternatively, if you need
non-blocking UI, implement a single-writer queue or debounce that serializes
calls to saveNotesToDisk() instead of fire-and-forget.
---
Nitpick comments:
In `@src/lib/features/note/view/focus_screen.dart`:
- Line 118: Replace the hardcoded external avatar URL usage in the CircleAvatar
widget (where NetworkImage('https://i.pravatar.cc/...') is used) with robust
handling: either switch to a local asset image placeholder or wrap the
NetworkImage usage with an Image widget that provides an errorBuilder/fallback
so a local placeholder displays when the network image fails; update the
CircleAvatar to use that Image provider or AssetImage and ensure the placeholder
asset is referenced (e.g., AssetImage or a named local asset) and included in
pubspec so the avatar never shows a broken image.
In `@src/lib/features/note/view/focus_widget.dart`:
- Around line 306-314: The current remove control uses a GestureDetector
wrapping a Container (calling vm.removeNote(note.id)) without accessibility
semantics; update this by replacing or augmenting the GestureDetector/Container
with an accessible widget (e.g., IconButton) or wrap it with a Semantics widget
and provide a meaningful label/tooltip like "Remove note" and an appropriate
onTap/action hint; ensure the handler still calls vm.removeNote(note.id) so
behavior is unchanged and that accessibility/semantic properties (label,
buttonRole) are set for screen readers.
- Line 20: Replace all uses of the deprecated Color.withOpacity(...) in this
widget with Color.withValues(alpha: ...). Specifically, update the five
occurrences inside the FocusWidget (the boxShadow entry and the four other
Color.withOpacity(...) calls around the build/layout code) to use
withValues(alpha: <same numeric>) to preserve precision and
forward-compatibility with Flutter 3.27+; ensure you keep the same numeric alpha
values and import stays unchanged.
In `@src/lib/features/note/viewmodel/focus_viewmodel.dart`:
- Around line 49-60: The catch block in pickImage currently only logs errors
(debugPrint) and gives no user feedback; modify the method signature to accept a
BuildContext (e.g., Future<void> pickImage(BuildContext context)) and, in the
catch(e) branch, show a user-visible message (for example via
ScaffoldMessenger.of(context).showSnackBar or a toast) indicating image
selection failed, while keeping the existing behavior of not changing
selectedImagePath and calling notifyListeners only on success; reference:
pickImage, _picker, selectedImagePath, notifyListeners, debugPrint.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62c1cafa-8f5d-43fa-936d-f90aefd440d8
📒 Files selected for processing (4)
src/lib/features/note/model/note_model.dartsrc/lib/features/note/view/focus_screen.dartsrc/lib/features/note/view/focus_widget.dartsrc/lib/features/note/viewmodel/focus_viewmodel.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/features/note/model/note_model.dart
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <[email protected]>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lib/features/note/viewmodel/focus_viewmodel.dart (1)
154-154:⚠️ Potential issue | 🟡 MinorGuard against non-positive durations in progress getter and updateSettings.
The
progressgetter divides bytotalTimewithout guarding against zero or negative values. SinceupdateSettingsis a public API that accepts arbitrary integers, invalid inputs can produceNaN/Infinityand break UI rendering.🛡️ Proposed fix
- double get progress => timeRemaining / totalTime; + double get progress => totalTime <= 0 ? 0.0 : (timeRemaining / totalTime).clamp(0.0, 1.0); void updateSettings({required int newPomodoroMinutes, required int newBreakMinutes, required bool vibrate, required int ringtone}) { + if (newPomodoroMinutes <= 0 || newBreakMinutes <= 0) { + throw ArgumentError('Timer durations must be greater than 0 minutes'); + } stopAlarm(); pomodoroTime = newPomodoroMinutes * 60;Also applies to: 223-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/note/viewmodel/focus_viewmodel.dart` at line 154, The progress getter currently returns timeRemaining / totalTime with no guard; modify the progress getter to return 0.0 (or clamp) when totalTime <= 0 to avoid NaN/Infinity and ensure it clamps to the 0.0–1.0 range (e.g., max(0.0, min(1.0, timeRemaining / totalTime))). In updateSettings (the public API that sets totalTime/timeRemaining), validate and sanitize inputs: reject or normalize non-positive totalTime (e.g., set a minimum of 1 or ignore the update), clamp timeRemaining into 0..totalTime, and ensure stored totalTime is positive so subsequent calls to the progress getter are safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/lib/features/note/viewmodel/focus_viewmodel.dart`:
- Line 154: The progress getter currently returns timeRemaining / totalTime with
no guard; modify the progress getter to return 0.0 (or clamp) when totalTime <=
0 to avoid NaN/Infinity and ensure it clamps to the 0.0–1.0 range (e.g.,
max(0.0, min(1.0, timeRemaining / totalTime))). In updateSettings (the public
API that sets totalTime/timeRemaining), validate and sanitize inputs: reject or
normalize non-positive totalTime (e.g., set a minimum of 1 or ignore the
update), clamp timeRemaining into 0..totalTime, and ensure stored totalTime is
positive so subsequent calls to the progress getter are safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0bbf105c-6993-4b41-b62d-89b010febacb
📒 Files selected for processing (3)
src/lib/features/note/view/focus_widget.dartsrc/lib/features/note/viewmodel/focus_viewmodel.dartsrc/pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pubspec.yaml
This was referenced Apr 9, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Changes
Chores